Upgrade Nuget Packages #3982
Upgrade Nuget Packages #3982
Conversation
Jack251970
added
dependencies
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughUpdates package versions (Meziantou.Framework.Win32.Jobs, Squirrel.Windows) and lockfiles; adjusts post-build script to use Squirrel 1.9.0. Centralizes the plugin deletion marker as Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Plugin UI
participant PM as PluginManager
participant FS as FileSystem
participant PC as PluginConfig
participant App as AppStartup
User->>UI: Request uninstall plugin
UI->>PM: Uninstall(pluginId)
PM->>FS: Write file DataLocation.PluginDeleteFile in plugin folder
Note right of PM: marker = "NeedDelete.txt"
App->>PC: Load plugins on startup
PC->>FS: Check Path.Combine(DataLocation.PortableDataPath/PluginPath, DataLocation.PluginDeleteFile)
alt marker exists
PC->>FS: Delete plugin folder
else marker missing
PC-->>App: Keep plugin
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
Flow.Launcher.Core/Flow.Launcher.Core.csproj (2)
1-1: Drop the BOM to avoid churn in diffs.The XML starts with a BOM (shows up as an invisible char before ). Safe to remove for consistency across csproj files.
-<Project Sdk="Microsoft.NET.Sdk"> +<Project Sdk="Microsoft.NET.Sdk">
62-64: Squirrel.Windows 1.9.0 pin: keep the rationale discoverable.Good call to pin and suppress NU1701. Please add a link to the upstream release/issue that dropped UpdateManager so future upgrades aren’t accidental.
- <!-- Do not upgrade this to higher version since higher version removes UpdateManager instance --> + <!-- Do not upgrade beyond 1.9.0: later versions remove UpdateManager. + Ref: <link to upstream release/issue> -->Flow.Launcher/packages.lock.json (1)
583-633: NETStandard.Library 1.6.1 and many System. shims added.*Expected side‑effect of Squirrel. Please sanity‑check runtime for binding conflicts on a clean machine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Flow.Launcher.Core/Flow.Launcher.Core.csproj(2 hunks)Flow.Launcher.Core/packages.lock.json(9 hunks)Flow.Launcher/packages.lock.json(17 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-08T15:52:58.573Z
Learnt from: taooceros
PR: Flow-Launcher/Flow.Launcher#2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Applied to files:
Flow.Launcher.Core/Flow.Launcher.Core.csprojFlow.Launcher.Core/packages.lock.jsonFlow.Launcher/packages.lock.json
📚 Learning: 2025-09-04T11:52:29.096Z
Learnt from: jjw24
PR: Flow-Launcher/Flow.Launcher#3932
File: Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs:48-55
Timestamp: 2025-09-04T11:52:29.096Z
Learning: In Flow Launcher's PluginsManifest.cs, when dealing with version parsing for the MinimumAppVersion feature, maintainer jjw24 prefers to keep the solution simple rather than implementing comprehensive helper methods for SemVer parsing normalization.
Applied to files:
Flow.Launcher.Core/Flow.Launcher.Core.csproj
🪛 GitHub Check: Check Spelling
Flow.Launcher.Core/packages.lock.json
[warning] 345-345:
osx is not a recognized word. (unrecognized-spelling)
[warning] 344-344:
opensuse is not a recognized word. (unrecognized-spelling)
[warning] 340-340:
debian is not a recognized word. (unrecognized-spelling)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (9)
Flow.Launcher.Core/Flow.Launcher.Core.csproj (2)
59-59: Meziantou.Framework.Win32.Jobs → 3.4.5 looks good.Patch update, no API surface here. 👍
62-64: Transitive bloat from legacy Squirrel (NETStandard.Library 1.6.1 + System. 4.3.0). Verify no runtime binding clashes.*The pin pulls in many netstandard shims. On net9.0-windows this usually resolves, but please run an app smoke test to ensure no FileLoadException/assembly mismatch at runtime.
Flow.Launcher.Core/packages.lock.json (4)
22-25: Lock: Meziantou.Framework.Win32.Jobs updated correctly.
40-48: Lock: Squirrel.Windows 1.9.0 and deps look consistent.DeltaCompressionDotNet bumped to 1.1.0; SharpCompress 0.17.1 introduced.
203-253: Large NETStandard.Library 1.6.1 block added via Squirrel.Expected with legacy packages. No action, just noting the source of the expanded System.* graph.
Ensure installer size/regression numbers are captured in PR notes.
392-399: Verify SharpCompress 0.17.1 and related packages for security advisoriesSharpCompress 0.17.1 (Flow.Launcher.Core/packages.lock.json, lines 392–399) is outdated and may have CVEs; scanners can flag it even if only used by Squirrel.Windows. Automated GitHub advisory lookup failed (gh GraphQL parse error). Manually check advisories for SharpCompress, Squirrel.Windows, and DeltaCompressionDotNet and upgrade to patched versions if any vulnerabilities are reported.
Flow.Launcher/packages.lock.json (3)
181-183: DeltaCompressionDotNet 1.1.0 syncs with Core lock.
250-254: Meziantou.Framework.Win32.Jobs 3.4.5 reflected at app level.
795-805: Squirrel.Windows 1.9.0 transitive entry matches Core.SharpCompress 0.17.1 is present here too; see advisory check comment in Core lockfile.
|
Two things I would like to test also with the squirrel upgrade:
Point 2 is a bit finicky (and been a long time since I tested this process locally), iirc you can build a version 1.0.0 locally and then test the update that way. |
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades two NuGet packages: Meziantou.Framework.Win32.Jobs from 3.4.4 to 3.4.5 and squirrel.windows from 1.5.2 to 1.9.0. The squirrel.windows upgrade provides several improvements including fixes, installation progress features, and reduced installer size.
- Updated NuGet package references in project files
- Modified build scripts to reference new squirrel.windows version
- Package lock files updated to reflect new transitive dependencies
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Scripts/post_build.ps1 | Updated hardcoded paths to reference squirrel.windows 1.9.0 tools |
| Flow.Launcher/packages.lock.json | Updated package lock with new versions and transitive dependencies |
| Flow.Launcher.Core/packages.lock.json | Updated package lock with new versions and transitive dependencies |
| Flow.Launcher.Core/Flow.Launcher.Core.csproj | Updated package references to new versions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
I actually remember higher version of squirrel does not work well with flow (since it is not using .net framework)? |
Yes. 1.9.0 is the highest version we can use. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Flow.Launcher.Core/Plugin/PluginManager.cs (1)
329-331: Use internal PluginModified helper in PluginManager.csReplace API.PluginModified(...) with the class's internal PluginModified(...) to avoid API indirection.
- if (API.PluginModified(plugin.Metadata.ID)) + if (PluginModified(plugin.Metadata.ID)) return Array.Empty<PluginPair>();rg shows other API.PluginModified usages (PluginInstaller, DialogJump, plugin-host managers); do not change those — they're outside PluginManager.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Flow.Launcher.Core/Plugin/PluginManager.cs(2 hunks)Flow.Launcher.Infrastructure/UserSettings/DataLocation.cs(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-01T05:46:13.251Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Applied to files:
Flow.Launcher.Core/Plugin/PluginManager.cs
🧬 Code graph analysis (2)
Flow.Launcher.Infrastructure/UserSettings/DataLocation.cs (1)
Flow.Launcher.Infrastructure/Constant.cs (1)
Constant(7-59)
Flow.Launcher.Core/Plugin/PluginManager.cs (2)
Flow.Launcher.Infrastructure/Constant.cs (1)
Constant(7-59)Flow.Launcher.Infrastructure/UserSettings/DataLocation.cs (1)
DataLocation(8-45)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
Flow.Launcher.Infrastructure/UserSettings/DataLocation.cs (4)
33-36: LGTM: computed properties for core directories.On-demand computation keeps paths consistent with dynamic
DataDirectory(). No behavior change expected.Please confirm any directory-creation call sites still run early (e.g.,
Directory.CreateDirectory(DataLocation.PluginsDirectory)in PluginManager’s static ctor) so these paths exist before use.
38-40: LGTM: plugin subdirectories as properties.Matches prior semantics and current consumers (
UpdatePluginDirectory).
45-45: LGTM:PluginEnvironmentsPathas a property.Consistent with the new pattern.
4-4: Narrow CA2211 suppression or remove it by eliminating public static mutable fields.File: Flow.Launcher.Infrastructure/UserSettings/DataLocation.cs — convert PortableDataPath/RoamingDataPath to get-only properties (preferred) and remove the pragma; otherwise limit the pragma scope to the two fields.
Before applying Option A, verify there are no assignments to these fields. Run locally:
rg -nP '\bDataLocation\.(PortableDataPath|RoamingDataPath)\s*=' -C2 rg -nP '\b(PortableDataPath|RoamingDataPath)\s*=' -C2 rg -nP '\bDataLocation\.(PortableDataPath|RoamingDataPath)\b' -C2If no matches, apply Option A; if any assignment exists, either update those sites or keep the fields and restore the pragma only around them (Option B).
Flow.Launcher.Core/Plugin/PluginManager.cs (1)
50-53: No change required — collection-expression[]is supported.All projects target net9.0, and .NET 9's default C# language version is C# 13, while collection expressions were introduced in C# 12 — keep the
[]syntax as-is. (learn.microsoft.com)
Jack251970
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Flow.Launcher.Infrastructure/UserSettings/DataLocation.cs (1)
24-29: Compilation blocker: missing)inifand stray control flow.Current code won’t compile. Simplify to a single-return expression.
- public static bool PortableDataLocationInUse() - { - if (Directory.Exists(PortableDataPath) && - !File.Exists(Path.Combine(PortableDataPath, DeletionIndicatorFile)) - return true; - - return false; - } + public static bool PortableDataLocationInUse() + { + return Directory.Exists(PortableDataPath) && + !File.Exists(Path.Combine(PortableDataPath, DeletionIndicatorFile)); + }
🧹 Nitpick comments (1)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (1)
125-145: Make PortableMode setter idempotent (usevalueinstead of unconditional toggle).Ignoring
valuecan flip modes even when the binding writes the same value. Use the target state to avoid unintended toggles and make the setter safe to re‑apply.- public bool PortableMode + public bool PortableMode { - get => _portableMode; + get => _portableMode; set { if (!_portable.CanUpdatePortability()) return; - if (DataLocation.PortableDataLocationInUse()) - { - _portable.DisablePortableMode(); - } - else - { - _portable.EnablePortableMode(); - } + var isPortable = DataLocation.PortableDataLocationInUse(); + if (value && !isPortable) + _portable.EnablePortableMode(); + else if (!value && isPortable) + _portable.DisablePortableMode(); + // else: no-op (already in desired state) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Flow.Launcher.Core/Configuration/Portable.cs(1 hunks)Flow.Launcher.Core/Plugin/PluginConfig.cs(2 hunks)Flow.Launcher.Core/Plugin/PluginManager.cs(3 hunks)Flow.Launcher.Infrastructure/UserSettings/DataLocation.cs(3 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Flow.Launcher.Core/Plugin/PluginManager.cs
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-07-01T05:46:13.251Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Applied to files:
Flow.Launcher.Core/Plugin/PluginConfig.cs
📚 Learning: 2025-07-21T09:19:49.684Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3854
File: Flow.Launcher/App.xaml.cs:246-262
Timestamp: 2025-07-21T09:19:49.684Z
Learning: In Flow Launcher's App.xaml.cs, the asynchronous plugin initialization task (containing AbstractPluginEnvironment.PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, and AutoPluginUpdates) does not require additional try-catch error handling according to maintainer Jack251970, as these operations are designed to handle exceptions internally.
Applied to files:
Flow.Launcher.Core/Plugin/PluginConfig.cs
📚 Learning: 2025-09-04T11:52:29.096Z
Learnt from: jjw24
PR: Flow-Launcher/Flow.Launcher#3932
File: Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs:48-55
Timestamp: 2025-09-04T11:52:29.096Z
Learning: In Flow Launcher's PluginsManifest.cs, when dealing with version parsing for the MinimumAppVersion feature, maintainer jjw24 prefers to keep the solution simple rather than implementing comprehensive helper methods for SemVer parsing normalization.
Applied to files:
Flow.Launcher.Core/Plugin/PluginConfig.cs
📚 Learning: 2025-07-21T09:19:19.012Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3854
File: Flow.Launcher.Core/Plugin/PluginManager.cs:280-292
Timestamp: 2025-07-21T09:19:19.012Z
Learning: In Flow Launcher's PluginManager.cs, the post-initialization operations (RegisterResultsUpdatedEvent, UpdatePluginMetadataTranslation, RegisterPluginActionKeywords, DialogJump.InitializeDialogJumpPlugin, and AddPluginToLists) are designed to be exception-safe and do not require additional try-catch error handling according to the maintainer Jack251970.
Applied to files:
Flow.Launcher.Core/Plugin/PluginConfig.cs
📚 Learning: 2024-11-03T07:40:11.014Z
Learnt from: Yusyuriv
PR: Flow-Launcher/Flow.Launcher#3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:40:11.014Z
Learning: In Flow Launcher, when using Windows Forms dialogs (e.g., in `JsonRPCPluginSettings.cs`), path validation is enabled by default in `OpenFileDialog` and `FolderBrowserDialog`, preventing users from selecting invalid paths, but it's possible to opt out of this validation on individual dialogs.
Applied to files:
Flow.Launcher.Core/Plugin/PluginConfig.cs
🧬 Code graph analysis (4)
Flow.Launcher.Core/Plugin/PluginConfig.cs (1)
Flow.Launcher.Infrastructure/UserSettings/DataLocation.cs (1)
DataLocation(8-47)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (1)
Flow.Launcher.Infrastructure/UserSettings/DataLocation.cs (2)
DataLocation(8-47)PortableDataLocationInUse(22-29)
Flow.Launcher.Infrastructure/UserSettings/DataLocation.cs (1)
Flow.Launcher.Infrastructure/Constant.cs (1)
Constant(7-59)
Flow.Launcher.Core/Configuration/Portable.cs (1)
Flow.Launcher.Infrastructure/UserSettings/DataLocation.cs (1)
DataLocation(8-47)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (1)
126-126: Caching startup portable state is reasonable.The static readonly snapshot aligns with “restart required” semantics.
Flow.Launcher.Core/Plugin/PluginConfig.cs (2)
9-10: Good: centralizing deletion marker dependency.Adding
using Flow.Launcher.Infrastructure.UserSettings;to sourcePluginDeleteFilefrom a single place reduces drift.
34-44: Resolved — PluginDeleteFile constant used; no hard-coded "NeedDelete.txt" occurrences.DataLocation.cs:45 defines PluginDeleteFile = "NeedDelete.txt"; usages at Flow.Launcher.Core/Plugin/PluginConfig.cs:34 and Flow.Launcher.Core/Plugin/PluginManager.cs:818.
Flow.Launcher.Core/Configuration/Portable.cs (1)
142-144: Good: rely onDataLocationfor paths in pre-start cleanup.This keeps path logic single-sourced and consistent with the rest of the codebase.
Flow.Launcher.Infrastructure/UserSettings/DataLocation.cs (1)
34-41: Good: computed directory properties and newPluginDeleteFileconstant.This aligns consumers and removes duplication.
Also applies to: 45-47
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
…low.Launcher into squirrel_upgrade
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@jjw24 Switching from installed to portable version and back now works correctly. (In this PR, I have resolved a portable mode bug so that it can work well) I have not tested updating current Flow version and I think it is unnecessary. Other things work well for me. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Flow.Launcher.Core/Plugin/PluginManager.cs (1)
817-823: Handle marker creation failures; log and avoid silent partial uninstall.If creating the deletion marker fails (locked dir, permissions), the old plugin won’t be removed on next start and you get duplicate versions. Wrap in try/catch and log.
- // Marked for deletion. Will be deleted on next start up - using var _ = File.CreateText(Path.Combine(plugin.PluginDirectory, DataLocation.PluginDeleteFile)); + // Marked for deletion. Will be deleted on next start up + try + { + var markerPath = Path.Combine(plugin.PluginDirectory, DataLocation.PluginDeleteFile); + File.WriteAllText(markerPath, string.Empty); + } + catch (Exception e) + { + API.LogException(ClassName, $"Failed to create deletion marker for <{plugin.Name}> at <{plugin.PluginDirectory}>", e); + // intentionally continue; user can manually delete if necessary + }
🧹 Nitpick comments (6)
Flow.Launcher.Core/Flow.Launcher.Core.csproj (1)
1-1: Stray BOM at file start (non-functional).There’s a BOM/invisible char before . Safe to keep, but consider removing to avoid diff churn.
Flow.Launcher/packages.lock.json (2)
4-4: App targets net9.0-windows10.0.19041 while Core targets windows7.0.If intentional, consider adding a brief note in the solution docs to prevent future “helpful” unification.
1597-1608: CI suggestion: enforce locked restores.To keep restore deterministic with the expanded graph, run dotnet restore --locked-mode in CI for all projects.
Scripts/post_build.ps1 (1)
34-37: Pin to 1.9.0 looks correct; add path validation and use Join-Path/LiteralPath for robustness.Hard-coding the NuGet cache path works on most Windows agents, but it fails silently when the package isn’t restored or the path shape changes. Also prefer Join-Path and -LiteralPath to avoid issues with spaces and backslashes.
Apply this diff to make the copy resilient:
function Copy-Resources ($path) { # making version static as multiple versions can exist in the nuget folder and in the case a breaking change is introduced. - Copy-Item -Force $env:USERPROFILE\.nuget\packages\squirrel.windows\1.9.0\tools\Squirrel.exe $path\Output\Update.exe + $squirrelExe = Join-Path $env:USERPROFILE ".nuget\packages\squirrel.windows\1.9.0\tools\Squirrel.exe" + if (-not (Test-Path -LiteralPath $squirrelExe)) { + throw "Squirrel.exe not found at '$squirrelExe'. Ensure squirrel.windows 1.9.0 is restored (nuget restore/dotnet restore)." + } + $dest = Join-Path $path "Output\Update.exe" + Copy-Item -LiteralPath $squirrelExe -Destination $dest -Force -ErrorAction Stop }Optional (outside this hunk): define a single version constant once to avoid future double-edits.
# near the top of the script $SquirrelVersion = '1.9.0'Then replace the hard-coded version in both places with
$SquirrelVersion.Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (1)
126-145: Snapshotting portable mode is fine; clarify setter semantics or switch to a command.The setter ignores
valueand toggles based on current state. Consider a TogglePortableMode command or a brief comment to avoid two‑way binding confusion.- public bool PortableMode + // Note: setter ignores 'value' and toggles state; restart required + public bool PortableModeFlow.Launcher.Core/Configuration/Portable.cs (1)
26-33: Update in-code comment to Squirrel 1.9.0 and preserve disposal guidanceFile: Flow.Launcher.Core/Configuration/Portable.cs (lines 26–33)
Verified: the 3‑arg UpdateManager ctor (string urlOrPath, string applicationName = null, string rootDirectory = null) exists in Squirrel.Windows 1.9.0; UpdateManager implements IDisposable and must be disposed — using() is acceptable but avoid awaiting async UpdateManager calls inside a using (can leak the mutex); CreateShortcutsForExecutable, RemoveShortcutsForExecutable, CreateUninstallerRegistryEntry and RemoveUninstallerRegistryEntry are present. Update the comment to reference 1.9.0 and add a short sentence about the awaiting‑inside‑using disposal gotcha or recommend explicit Dispose/ProcessExit handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
Flow.Launcher.Core/Configuration/Portable.cs(1 hunks)Flow.Launcher.Core/Flow.Launcher.Core.csproj(2 hunks)Flow.Launcher.Core/Plugin/PluginConfig.cs(2 hunks)Flow.Launcher.Core/Plugin/PluginManager.cs(2 hunks)Flow.Launcher.Core/packages.lock.json(9 hunks)Flow.Launcher.Infrastructure/UserSettings/DataLocation.cs(3 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs(1 hunks)Flow.Launcher/packages.lock.json(17 hunks)Scripts/post_build.ps1(2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-07-01T05:46:13.251Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Applied to files:
Flow.Launcher.Core/Plugin/PluginManager.csFlow.Launcher.Core/Plugin/PluginConfig.cs
📚 Learning: 2024-10-08T15:52:58.573Z
Learnt from: taooceros
PR: Flow-Launcher/Flow.Launcher#2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Applied to files:
Flow.Launcher.Core/Flow.Launcher.Core.csprojFlow.Launcher.Core/packages.lock.jsonFlow.Launcher/packages.lock.json
📚 Learning: 2025-09-04T11:52:29.096Z
Learnt from: jjw24
PR: Flow-Launcher/Flow.Launcher#3932
File: Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs:48-55
Timestamp: 2025-09-04T11:52:29.096Z
Learning: In Flow Launcher's PluginsManifest.cs, when dealing with version parsing for the MinimumAppVersion feature, maintainer jjw24 prefers to keep the solution simple rather than implementing comprehensive helper methods for SemVer parsing normalization.
Applied to files:
Flow.Launcher.Core/Flow.Launcher.Core.csprojFlow.Launcher.Core/Plugin/PluginConfig.cs
📚 Learning: 2025-07-21T09:19:49.684Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3854
File: Flow.Launcher/App.xaml.cs:246-262
Timestamp: 2025-07-21T09:19:49.684Z
Learning: In Flow Launcher's App.xaml.cs, the asynchronous plugin initialization task (containing AbstractPluginEnvironment.PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, and AutoPluginUpdates) does not require additional try-catch error handling according to maintainer Jack251970, as these operations are designed to handle exceptions internally.
Applied to files:
Flow.Launcher.Core/Plugin/PluginConfig.cs
📚 Learning: 2025-07-21T09:19:19.012Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3854
File: Flow.Launcher.Core/Plugin/PluginManager.cs:280-292
Timestamp: 2025-07-21T09:19:19.012Z
Learning: In Flow Launcher's PluginManager.cs, the post-initialization operations (RegisterResultsUpdatedEvent, UpdatePluginMetadataTranslation, RegisterPluginActionKeywords, DialogJump.InitializeDialogJumpPlugin, and AddPluginToLists) are designed to be exception-safe and do not require additional try-catch error handling according to the maintainer Jack251970.
Applied to files:
Flow.Launcher.Core/Plugin/PluginConfig.cs
📚 Learning: 2024-11-03T07:40:11.014Z
Learnt from: Yusyuriv
PR: Flow-Launcher/Flow.Launcher#3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:40:11.014Z
Learning: In Flow Launcher, when using Windows Forms dialogs (e.g., in `JsonRPCPluginSettings.cs`), path validation is enabled by default in `OpenFileDialog` and `FolderBrowserDialog`, preventing users from selecting invalid paths, but it's possible to opt out of this validation on individual dialogs.
Applied to files:
Flow.Launcher.Core/Plugin/PluginConfig.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: gitStream.cm
🔇 Additional comments (12)
Flow.Launcher.Core/Flow.Launcher.Core.csproj (2)
59-59: Jobs lib: minor upgrade looks good.Meziantou.Framework.Win32.Jobs 3.4.5 aligns with lockfiles. No concerns.
62-64: Approve — keep Squirrel.Windows pinned to 1.9.0 (NU1701); local verification requiredPin preserves UpdateManager API. Sandbox couldn't run checks (ripgrep skipped files; dotnet not installed). Run these locally and paste results in the PR:
# 1) Inventory Squirrel API usage rg -nP -g '!**/obj/**' -C2 '\b(UpdateManager|SquirrelAwareApp|IUpdateManager|ReleaseEntry)\b' --type cs # 2) Ensure restore uses lock file dotnet restore --locked-mode # 3) Manual smoke plan (document results in PR): # a) installed → portable → installed (verify settings/data preserved and no stale delete markers) # b) update flow: install an older local build, then upgrade to current dev via Squirrel # c) confirm update rollback path still works (if applicable)Flow.Launcher.Core/packages.lock.json (2)
20-25: Lock update: Meziantou.Framework.Win32.Jobs resolved to 3.4.5.Matches csproj. All good.
4-4: Different Windows TFMs across projects (7.0 here vs 10.0.19041 in app).Assuming Core doesn’t call Win10‑only APIs, this is fine. Please confirm the split is intentional to avoid accidental API usage drift.
Flow.Launcher/packages.lock.json (1)
181-183: Lock updates align with csproj changes (DeltaCompressionDotNet 1.1.0, Squirrel 1.9.0, Meziantou 3.4.5).Looks consistent. No functional concerns.
Also applies to: 797-805, 252-254
Scripts/post_build.ps1 (1)
82-82: Alias update is fine — compute the path and fail fast (or invoke the exe directly).File: Scripts/post_build.ps1 (around line 82) — compute $squirrelExe, Test-Path and throw if missing to avoid CI “command not found” failures.
- New-Alias Squirrel $env:USERPROFILE\.nuget\packages\squirrel.windows\1.9.0\tools\Squirrel.exe -Force + $squirrelExe = Join-Path $env:USERPROFILE ".nuget\packages\squirrel.windows\1.9.0\tools\Squirrel.exe" + if (-not (Test-Path -LiteralPath $squirrelExe)) { + throw "Squirrel.exe not found at '$squirrelExe'." + } + New-Alias -Name Squirrel -Value $squirrelExe -ForceAlternatively invoke directly:
& $squirrelExe --releasify $nupkg --releaseDir $temp --setupIcon $icon --no-msi | Write-OutputYour Test-Path run failed because it was executed in Bash. Re-run the check in PowerShell (pwsh or Windows PowerShell) and report the result:
Test-Path "$env:USERPROFILE.nuget\packages\squirrel.windows\1.9.0\tools\Squirrel.exe"
Flow.Launcher.Core/Plugin/PluginManager.cs (1)
329-331: Use internal PluginModified() instead of API.PluginModified().Per prior guidance, prefer the internal static check for consistency and to avoid the API hop.
- if (API.PluginModified(plugin.Metadata.ID)) + if (PluginModified(plugin.Metadata.ID)) return Array.Empty<PluginPair>();Flow.Launcher.Core/Plugin/PluginConfig.cs (1)
34-45: LGTM: unified plugin deletion marker.Switching to DataLocation.PluginDeleteFile aligns write/read of the marker across the codebase.
Flow.Launcher.Core/Configuration/Portable.cs (1)
142-144: Good: centralizing paths via DataLocation.Deriving both dirs from DataLocation removes duplication and matches the portability fix.
Flow.Launcher.Infrastructure/UserSettings/DataLocation.cs (3)
10-11: Immutability win.Making PortableDataPath/RoamingDataPath readonly removes a whole class of accidental mutations.
43-45: Consistent plugin marker constant — approved. Search for "NeedDelete.txt" in .cs files returns only Flow.Launcher.Infrastructure/UserSettings/DataLocation.cs:43 (the PluginDeleteFile definition); no other hard-coded occurrences found.
22-24: Bug fix: scope .dead check to PortableDataPathChecks the deletion-indicator inside PortableDataPath to prevent false-portable detection.
Location: Flow.Launcher.Infrastructure/UserSettings/DataLocation.cs:22-24
Confirm there are no remaining calls to File.Exists(DeletionIndicatorFile) that omit Path.Combine(PortableDataPath, ...).
jjw24
added
the
review in progress
Jack251970
changed the title
Upgrade Nuget Packages
Meziantou.Framework.Win32.Jobsfrom3.4.4to3.4.5squirrel.windowsfrom1.5.2to1.9.0Test
Restart Flow Launcher can work well
Memory usage
Original: 93.1MB, 97.1MB, 107.3MB
Upgraded: 89.1MB, 94.1MB, 97.1MB
Original: 611.2MB, 688.0MB, 694.2MB
Upgraded: 615.2MB, 692.0MB, 697.9MB
Portable mode can work correctly
Installer size: 93.6 -> 92MB